Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix velocity Verlet setter #3274

Closed
wants to merge 1 commit into from
Closed

Fix velocity Verlet setter #3274

wants to merge 1 commit into from

Conversation

jngrad
Copy link
Member

@jngrad jngrad commented Oct 23, 2019

Not for direct merging.

Fixes the bug found in #3271

When setting up a system with the NpT integrator, then switch to the VV integrator, the system keeps the NpT integrator in the core. To reproduce the error, take e.g. samples/visualization_npt.py and replace the call to system.integrator.set_isotropic_npt by the following:

system.integrator.set_isotropic_npt(ext_pressure=1.0, piston=0.01)
print(system.box_l)
system.integrator.set_vv()  # script interface: velocity Verlet, core: NpT
system.integrator.run(100)
print(system.box_l)
exit(0)  # exit now to avoid starting the visualizer

output:

[10. 10. 10.]
[10.00003915 10.00003915 10.00003915]

@jngrad
Fix velocity Verlet interface
2964e01 
When setting up the NPT integrator, then the VV integrator, the core
integrator remains on NPT. Since NVT and VV are the same, we can set
the core integrator to NVT when the interface is set to VV.
@jngrad jngrad added the BugFix label Oct 23, 2019
@jngrad jngrad added this to the Espresso 4.1.1 milestone Oct 23, 2019
@jngrad jngrad requested a review from RudolfWeeber October 23, 2019 16:25
@codecov
Copy link

codecov bot commented Oct 23, 2019 •
edited
Loading

Codecov Report

Merging #3274 into 4.1 will decrease coverage by <1%.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff          @@
##             4.1   #3274   +/-   ##
=====================================
- Coverage     85%     85%   -1%     
=====================================
  Files        530     530           
  Lines      26025   26025           
=====================================
- Hits       22157   22139   -18     
- Misses      3868    3886   +18
Impacted Files Coverage Δ
src/core/electrostatics_magnetostatics/p3m.cpp 84% <0%> (-2%) ⬇️
src/core/event.cpp 96% <0%> (ø) ⬆️
src/core/integrate.cpp 68% <0%> (+1%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3ffa392...2964e01. Read the comment docs.

@jngrad
Copy link
Member Author

jngrad commented Oct 30, 2019

Merged in 4.1.1 as 0865e1d

@jngrad jngrad closed this Oct 30, 2019
@jngrad jngrad mentioned this pull request Dec 24, 2019
Merged
kodiakhq bot added a commit that referenced this pull request Dec 24, 2019
@kodiakhq
Fix velocity Verlet interface (#3388)
364c0b4 
Fixes the bug found in #3271.
Same as #3274, but for the development branch.
@jngrad jngrad deleted the fix-3271-v411 branch January 18, 2022 12:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RudolfWeeber RudolfWeeber RudolfWeeber approved these changes

Assignees
No one assigned
Labels
BugFix
Projects
None yet
Milestone
Espresso 4.1.1
Development

Successfully merging this pull request may close these issues.
2 participants
@jngrad @RudolfWeeber